Add option to skip .repo files in the cloned repository #10
Conversation
| ARG ENABLE_RECURSIVE_CLONED="True" | ||
| RUN /usr/local/bin/recursive_vcs_import.py src src/upstream ${ENABLE_RECURSIVE_CLONED} |
There was a problem hiding this comment.
I suggest to fall back to just calling vcs import directly, if recursion is disabled. Somewhat goes against the naming of recursive_vcs_import.py otherwise.
| ARG ENABLE_RECURSIVE_CLONED="True" | |
| RUN /usr/local/bin/recursive_vcs_import.py src src/upstream ${ENABLE_RECURSIVE_CLONED} | |
| ARG ENABLE_RECURSIVE_CLONED="true" | |
| RUN if [[ $ENABLE_RECURSIVE_ADDITIONAL_DEBS == 'true' ]]; then \ | |
| /usr/local/bin/recursive_vcs_import.py src src/upstream ; \ | |
| else \ | |
| # vcs import ... ; \ | |
| fi |
There was a problem hiding this comment.
Is it possible to include every *.repo files in the current directory just using the vcs import command? I could see the benefit of a repository having for instance "hardware.repo", "vision.repo", etc. Thus, when I added the flag I was still interested in the recursive search, just not for the cloned packages.
There was a problem hiding this comment.
I have discussed this with my peers and we don't really see the necessity for multiple top-level .repo files that are all found. Either there is one that you would like to include, or you want to search recursively. The only additional argument that we could see being integrated is to have a variable for configuring the script to search for another filename, e.g., vision.repo instead of .repos. Similar to how you can configure CUSTOM_SCRIPT_FILE.
| @@ -30,6 +30,7 @@ build_image() { | |||
| $(if [[ -n "${ENABLE_RECURSIVE_ADDITIONAL_PIP}" ]]; then echo "--build-arg ENABLE_RECURSIVE_ADDITIONAL_PIP=${ENABLE_RECURSIVE_ADDITIONAL_PIP}"; fi) \ | |||
| $(if [[ -n "${CUSTOM_SCRIPT_FILE}" ]]; then echo "--build-arg CUSTOM_SCRIPT_FILE=${CUSTOM_SCRIPT_FILE}"; fi) \ | |||
| $(if [[ -n "${ENABLE_RECURSIVE_CUSTOM_SCRIPT}" ]]; then echo "--build-arg ENABLE_RECURSIVE_CUSTOM_SCRIPT=${ENABLE_RECURSIVE_CUSTOM_SCRIPT}"; fi) \ | |||
| $(if [[ -n "${ENABLE_RECURSIVE_CLONED}" ]]; then echo "--build-arg ENABLE_RECURSIVE_CLONED=${ENABLE_RECURSIVE_CLONED}"; fi) \ | |||
There was a problem hiding this comment.
I suggest to name the variable ENABLE_RECURSIVE_VCS_IMPORT.
It also still needs to be added to the README.
| @@ -30,6 +30,7 @@ build_image() { | |||
| $(if [[ -n "${ENABLE_RECURSIVE_ADDITIONAL_PIP}" ]]; then echo "--build-arg ENABLE_RECURSIVE_ADDITIONAL_PIP=${ENABLE_RECURSIVE_ADDITIONAL_PIP}"; fi) \ | |||
| $(if [[ -n "${CUSTOM_SCRIPT_FILE}" ]]; then echo "--build-arg CUSTOM_SCRIPT_FILE=${CUSTOM_SCRIPT_FILE}"; fi) \ | |||
| $(if [[ -n "${ENABLE_RECURSIVE_CUSTOM_SCRIPT}" ]]; then echo "--build-arg ENABLE_RECURSIVE_CUSTOM_SCRIPT=${ENABLE_RECURSIVE_CUSTOM_SCRIPT}"; fi) \ | |||
| $(if [[ -n "${ENABLE_RECURSIVE_CLONED}" ]]; then echo "--build-arg ENABLE_RECURSIVE_CLONED=${ENABLE_RECURSIVE_CLONED}"; fi) \ | |||
There was a problem hiding this comment.
The new variable must also be added to action.yml and .gitlab-ci.yml.
I am using it to create ready to deploy images of my workspace. Abstracting everything from the dockerbuild is quite an interesting idea, as it reduces the maintenance and can be easily added as a github action. |
|
Due to the many changes, we have decided to cleanly redo the PR (see #13). There, the changes you suggested have been taken into account. |
I was checking the final install folder and noticed some extra dependencies being built. This was due to some ros2 packages also including their own .repo files. In my case, these packages were already found and installed automatically by rosdep, thus there was no point in manually building them.
In order to keep the previous behavior, I have added a new variable to disable recursively checking the .repo files within the cloned repos. I have decided to keep the current script to still find every .repo files available when calling the build script.